Introduce ExpressiveFor and ExpressiveForConstructor attributes#4
Merged
Introduce ExpressiveFor and ExpressiveForConstructor attributes#4
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for mapping external methods/properties/constructors to generated expression trees via new [ExpressiveFor] / [ExpressiveForConstructor] attributes, enabling providers like EF Core to translate otherwise-nontranslatable calls after ExpandExpressives().
Changes:
- Introduce
[ExpressiveFor]and[ExpressiveForConstructor]attributes and generator support (interpretation, emission, registry entries). - Extend runtime resolution/replacement to locate external mappings across loaded registries.
- Add generator + integration tests demonstrating external method usage (EF Core + expression-compile runners).
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ExpressiveSharp.Tests/Services/ExpressiveReplacerTests.cs | Updates test resolver stub to support external-expression lookup. |
| tests/ExpressiveSharp.IntegrationTests/Scenarios/Store/Models/PricingUtils.cs | Adds “external” utility methods plus [ExpressiveFor] mapping stubs. |
| tests/ExpressiveSharp.IntegrationTests/Scenarios/Common/Tests/ExpressiveForMappingTests.cs | Scenario tests exercising expansion of mapped external calls. |
| tests/ExpressiveSharp.IntegrationTests.ExpressionCompile/Tests/Common/ExpressiveForMappingTests.cs | Runs the scenario tests against expression compilation runner. |
| tests/ExpressiveSharp.IntegrationTests.EntityFrameworkCore/Tests/Common/ExpressiveForMappingTests.cs | Runs the scenario tests against EF Core SQLite runner. |
| tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/RegistryTests.SingleProperty_RegistryContainsEntry.verified.txt | Updates expected registry emission to locate generated expression classes for external targets. |
| tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/RegistryTests.SingleMethod_RegistryContainsEntry.verified.txt | Same as above. |
| tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/RegistryTests.MultipleExpressives_AllRegistered.verified.txt | Same as above. |
| tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/RegistryTests.MethodOverloads_BothRegistered.verified.txt | Same as above. |
| tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExpressiveForTests.cs | Adds generator tests for [ExpressiveFor] scenarios + diagnostics. |
| tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExpressiveForTests.StaticProperty.verified.txt | Verified output for static-property mapping. |
| tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExpressiveForTests.StaticMethod.verified.txt | Verified output for static-method mapping. |
| tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExpressiveForTests.OverloadDisambiguation.verified.txt | Verified output for overload disambiguation. |
| tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExpressiveForTests.InstanceProperty.verified.txt | Verified output for instance-property mapping. |
| tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExpressiveForTests.InstanceMethod.verified.txt | Verified output for instance-method mapping. |
| src/ExpressiveSharp/Services/IExpressiveResolver.cs | Adds FindExternalExpression(MemberInfo) API. |
| src/ExpressiveSharp/Services/ExpressiveResolver.cs | Implements external lookup via scanning loaded registries and ambiguity detection. |
| src/ExpressiveSharp/Services/ExpressiveReplacer.cs | Uses resolver external lookup when member isn’t [Expressive]. |
| src/ExpressiveSharp/Mapping/ExpressiveForAttribute.cs | New attribute for external method/property mapping. |
| src/ExpressiveSharp/Mapping/ExpressiveForConstructorAttribute.cs | New attribute for external constructor mapping. |
| src/ExpressiveSharp.Generator/Registry/ExpressionRegistryEmitter.cs | Updates registry helper to find generated expression classes even when target member’s assembly differs. |
| src/ExpressiveSharp.Generator/Models/ExpressiveForAttributeData.cs | Adds immutable-ish snapshot model for [ExpressiveFor*] attribute args. |
| src/ExpressiveSharp.Generator/Interpretation/ExpressiveForInterpreter.cs | Resolves/validates external targets and builds descriptors from stub bodies. |
| src/ExpressiveSharp.Generator/Infrastructure/Diagnostics.cs | Adds diagnostics for [ExpressiveFor*] validation. |
| src/ExpressiveSharp.Generator/ExpressiveGenerator.cs | Adds incremental pipelines for [ExpressiveFor] + [ExpressiveForConstructor] and merges registry entries. |
| src/ExpressiveSharp.Generator/Comparers/ExpressiveForMemberCompilationEqualityComparer.cs | Adds comparer to support incremental caching for the new pipelines. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExpressiveForTests.cs
Outdated
Show resolved
Hide resolved
Comment on lines
+492
to
+509
| // Find the matching target method to get its parameter types (not the stub's) | ||
| var targetMethod = targetType.GetMembers(memberName).OfType<IMethodSymbol>() | ||
| .Where(m => m.MethodKind is not (MethodKind.PropertyGet or MethodKind.PropertySet)) | ||
| .FirstOrDefault(m => | ||
| { | ||
| var expectedParamCount = m.IsStatic ? m.Parameters.Length : m.Parameters.Length + 1; | ||
| if (stubSymbol.Parameters.Length != expectedParamCount) | ||
| return false; | ||
|
|
||
| var offset = m.IsStatic ? 0 : 1; | ||
| for (var i = 0; i < m.Parameters.Length; i++) | ||
| { | ||
| if (!SymbolEqualityComparer.Default.Equals( | ||
| m.Parameters[i].Type, stubSymbol.Parameters[i + offset].Type)) | ||
| return false; | ||
| } | ||
| return true; | ||
| }); |
There was a problem hiding this comment.
In ExtractRegistryEntryForExternal, the method-matching predicate compares target method parameter types to the stub parameters (with an offset), but it does not validate the receiver parameter type for instance methods. This can cause the registry entry to be generated for an invalid stub whose first parameter is not the target type. Add an explicit check for !m.IsStatic that stubSymbol.Parameters[0].Type equals targetType so registry extraction matches the intended stub rules.
Comment on lines
+122
to
+126
| private static void EnsureAllRegistriesLoaded() | ||
| { | ||
| if (_allRegistriesScanned) return; | ||
| _allRegistriesScanned = true; | ||
|
|
There was a problem hiding this comment.
EnsureAllRegistriesLoaded sets _allRegistriesScanned = true before the scan completes. A concurrent call can observe the flag and skip scanning while _assemblyRegistries is still incomplete, causing FindExternalExpression to miss mappings nondeterministically. Set the flag only after the scan finishes and guard the scan with a lock/LazyInitializer/Interlocked so other threads either participate safely or wait for completion.
src/ExpressiveSharp.Generator/Interpretation/ExpressiveForInterpreter.cs
Outdated
Show resolved
Hide resolved
src/ExpressiveSharp.Generator/Interpretation/ExpressiveForInterpreter.cs
Show resolved
Hide resolved
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'ExpressiveSharp Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: ffab74a | Previous: 0046f23 | Ratio |
|---|---|---|---|
ExpressiveSharp.Benchmarks.ExpressionReplacerBenchmarks.Replace_Property |
2162.444798787435 ns (± 435.0877750613811) |
1394.9290396372478 ns (± 5.448077193463971) |
1.55 |
ExpressiveSharp.Benchmarks.ExpressionResolverBenchmarks.Resolve_Property |
12.659543802340826 ns (± 0.00481655495068382) |
7.243337581555049 ns (± 0.004796838048350559) |
1.75 |
ExpressiveSharp.Benchmarks.ExpressionReplacerBenchmarks.Replace_Method |
1993.6999816894531 ns (± 563.3532983606609) |
1583.762092590332 ns (± 32.24036475542114) |
1.26 |
ExpressiveSharp.Benchmarks.ExpressionReplacerBenchmarks.Replace_BlockBody |
6225.739481608073 ns (± 2857.162931257167) |
3217.1149571736655 ns (± 448.34585470493164) |
1.94 |
ExpressiveSharp.Benchmarks.ExpressionReplacerBenchmarks.Replace_DeepChain |
10772.116088867188 ns (± 1368.8293881927432) |
7898.979766845703 ns (± 253.75951040334567) |
1.36 |
ExpressiveSharp.Benchmarks.GeneratorBenchmarks.RunGenerator(ExpressiveCount: 1) |
2744221.4244791665 ns (± 710999.7429314024) |
1299006.9557291667 ns (± 245120.90547145656) |
2.11 |
ExpressiveSharp.Benchmarks.GeneratorBenchmarks.RunGenerator_NoiseChange(ExpressiveCount: 1) |
2185990.9921875 ns (± 84715.94383242387) |
1378405.3072916667 ns (± 345322.72699958185) |
1.59 |
This comment was automatically generated by workflow using github-action-benchmark.
… for expression tree mapping
Document external member mapping in the README (after Expression Transformers) and update the migration guide to replace the "UseMemberBody removed — open an issue" guidance with a concrete migration path via [ExpressiveFor]. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use immutable arrays for TransformerTypeNames in attribute data snapshots to ensure incremental generator caching correctness - Rename misnamed test (TargetTypeNotFound_EXP0014 → MemberNotFound_EXP0015) - Remove unused EXP0020 diagnostic (declared but never reported) - Add receiver type validation in FindTargetProperty, FindTargetMethod, and ExtractRegistryEntryForExternal for instance members - Exclude indexers from property matching in FindTargetProperty - Fix race condition in EnsureAllRegistriesLoaded (double-checked lock, flag set after scan completes) - Replace fragile assembly name-prefix filter with IsDynamic check only Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…EXP0020) Two stubs targeting the same member produce generated classes with the same name. Previously this crashed the generator with a cryptic CS8785. Now: - ExpressionRegistryEntry carries a SourceLocation record struct for [ExpressiveFor] stubs (primitives only, safe for incremental caching) - ExpressionRegistryEmitter.Emit groups entries by GeneratedClassFullName and reports EXP0020 on each duplicate stub's source location - ExecuteFor collects items in a batch and skips duplicate emissions (per-item RegisterSourceOutput can't catch AddSource collisions since Roslyn deduplicates after all callbacks complete) Cross-project duplicates are still caught at runtime via the existing InvalidOperationException in FindExternalExpression. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Update ExtractRegistryEntryForExternal and ExecuteFor to use the new GenerateClassName/GenerateMethodSuffix API (partial classes per type, methods per member) - Fix EXP0020 duplicate detection to group by class+method (not just class name, which is now shared across methods of the same type) - Pass ExpressionMethodName in registry entries for external stubs - Update snapshot baselines for new generated code shape Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
koenbeuk
force-pushed
the
feat/proxied-expressives
branch
from
6c7bc71 to
ffab74a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
… for expression tree mapping